fix(service-automation): runRegion carries the outer region's index through nesting; the parallel branch index moves to branch (#15230) - #16367
Conversation
… change (#15230) Red-first. On this unmodified tree the three ruling pins read: loop { parallel } RED — branch=undefined on every branch step, and `iteration` carries the BRANCH index (0/1) instead of the row (0..2): the loop's index is discarded. bare parallel RED — expected undefined to be +0 at `branch`. loop { try_catch } GREEN — the control arm; try/catch already forwards the enclosing loop's iteration at its own call site. spec refusal GREEN — `branch` already refuses -1 / 1.5 at the `branch` path (the contract half landed with PR #15227). The type-level pin is invisible to vitest (esbuild strips types) and is measured by tsc: TS2344 `Type '"branch"' is not assignable to type 'keyof StepLogEntry'`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…hrough nesting; the parallel branch index moves to `branch` (#15230) The engine half of the maintainer ruling of 2026-09-03; the contract half (`ExecutionStepLogSchema.branch`) already landed in `@objectstack/spec`. `runRegion`'s tagger let the innermost region win outright, skipping any step a nested region had already tagged, and `parallel` wrote its branch index into `iteration`. Together those made `loop { parallel }` unreadable: every branch step recorded its branch and no step of that branch recorded its row. The tagger now splits what "innermost wins" governs. IDENTITY fields (`parentNodeId` / `regionKind` / `retryAttempt`) answer WHICH REGION ran the step and still belong to the innermost region outright. INDEX fields (`iteration` / `branch`) answer WHICH PASS of which region — nested regions contribute different ones, both true of the same step — so an enclosing region fills the index the inner one left undefined instead of being skipped along with the identity fields. Still "fill only what is undefined", so `loop { loop }` keeps the inner loop's `iteration`. `try` / `catch` inside a loop is unchanged (the control arm): such a region has no index of its own, so its steps keep the loop's `iteration` and gain no `branch`. Two fixtures pinned the retired branch and are replaced, not re-spelled: parallel-node.test.ts asserted the branch index ON `iteration` for a bare parallel, and contained-failure-visibility.test.ts carried the "#14414 fence" asserting that #14456 had left the overload standing. #15230 is the card that retires it, so the fence is spent and now reads the two indices apart. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…15230) The clause's `fixtures.requires` said "filed separately"; the card is #16356. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
…allel } fixture note (#15230) "sit unmeasured through two releases" asserted a span I did not measure. The point the note makes — that a missing clause is what let the overload go unmeasured — stands without it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
📓 Docs Drift CheckThis PR changes 1 package(s): ⛔ 2 release-owned page(s) name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 5 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5b522257023f165cbb210a252e2ad958d04e4872 && git checkout 5b522257023f165cbb210a252e2ad958d04e4872
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 055848ee708d13bb0839cac39b340103239ac781 93fa7157bed23e5639b18f5c0d70d83919a26997 && git checkout -B drift-repro 055848ee708d13bb0839cac39b340103239ac781 && git merge --no-ff 93fa7157bed23e5639b18f5c0d70d83919a26997
node scripts/docs-audit/affected-docs.mjs --json 055848ee708d13bb0839cac39b340103239ac781
|
Fixes #15230
The engine half of the maintainer ruling of 2026-09-03 (recorded on #14414). The contract half —
ExecutionStepLogSchema.branch, anditerationre-described as single-valued — landed with PR #15227 and is untouched here.The defect
Two facts combined to make
loop { body: [ parallel { branches } ] }unreadable.parallelwrote its branch index intoiteration, the same keyloopwrites its row index into — one field, two meanings, told apart only by readingregionKindfirst. AndrunRegion's tagger let the innermost region win outright: a step that already carried aparentNodeIdwas skipped entirely, so an enclosing region contributed nothing to it.Together: every branch step of a
loop { parallel }recorded its branch index, and no step of that branch recorded the row it ran for. A per-row failure inside a branch was attributable to a branch and never to a row — which is precisely the shape a fan-out inside a scheduled sweep has, and the one an operator most needs to read.Measured on the unmodified tree, three rows x two branches:
All three of
leafA's rows reportiteration=0and all three ofleafB's reportiteration=1: that is the BRANCH index, constant per branch. The row index — 0, 1, 2 — appears nowhere, andbranchis never written at all.The change
runRegion's tagger splits what "innermost wins" governs. The two halves answer different questions:parentNodeId,regionKind,retryAttempt— answers which region ran this step. Innermost wins outright, exactly as before: a step a nested region already claimed keeps naming that region, and an enclosing region never relabels it.iteration,branch— answers which pass of which region. Nested regions contribute different indices that are both true of the same step, so an enclosing region now fills the index the inner region left undefined instead of being turned away with the identity fields.It is still "fill only what is undefined" on both halves, so an index a nested region did set wins:
loop { loop }keeps the inner loop'siteration, unchanged.parallelwritesbranch: i, notiteration: i(builtin/parallel-node.ts). This file is beyond the surface the card named, and unavoidably so: the tagger writes whatgroupinghands it, and the branch index is produced at this call site. The alternative — having the tagger key onregionKind === 'parallel-branch'and re-home the value — puts implicit magic in the shared tagger to keep one file out of the diff.StepLogEntrygainsbranch?: numberand the overloaded comment is rewritten to one meaning per key. The interface is exported, and the change is exactly one optional output key, so the dispatch'sClause-②: nostop condition ("more than one optional output key") is not met.try/catchinside a loop is unchanged, deliberately — the control arm, not a subject. Such a region has no index of its own, so its steps keep carrying the enclosing loop'siterationwithregionKindstill naming the region, and gain nobranch.builtin/try-catch-node.tsis in the diff for its comment only: the comment asserted "a loop's own tagger can never reach past this one to a try/catch step", which this change makes false. The forwarding it explains stays (same value, and it is what$error.iterationis bound from); only the claim about the tagger is corrected.Pins
packages/services/service-automation/src/builtin/region-index-keys.test.ts— the ruling's own three cases, plus two guards, plus a type-level pin.Red-first, with the predictions written before the run (they held exactly):
loop { parallel }— both indices on one stepbranch=undefined,iteration= the branch indexparallel—branchwritten,iterationabsentexpected undefined to be +0loop { try_catch }— unchanged (control arm)ExecutionStepLogSchemabranchrefuses-1/1.5at thebranchpathThe last row is the card's "kept in step with the spec type by a pin, not by prose". It is a type-level assertion, so vitest cannot see it — esbuild strips types and a green
pnpm testsays nothing about it. It is measured bytsc, where on the unmodified tree it read:Both
tsconfig.jsonandtsconfig.test.jsonreachsrc/**with no test exclusion, so the pin is genuinely compiled — the errors above came out of that file, which is the positive proof.Two fixtures replaced, not re-spelled
Both pinned exactly the branch the ruling deleted, so re-spelling the assertion would have left the retired overload free to come back as a second writer.
builtin/parallel-node.test.tsasserted the branch index oniterationfor a bareparallel. It now assertsbranch, and assertsiterationis absent — the absence is the half that keeps the overload from returning.builtin/contained-failure-visibility.test.tscarried the "Aparallelbranch inside aloopbody overloads the step record'siterationwith the branch index — the enclosing loop iteration is lost, so a branch step cannot be attributed to its row #14414 fence", whose comment read "the pre-existing overload Aparallelbranch inside aloopbody overloads the step record'siterationwith the branch index — the enclosing loop iteration is lost, so a branch step cannot be attributed to its row #14414 owns. Nothing here changed it." This is the card that retires that overload, so the fence is spent; it now reads the two indices apart over its five rows x two branches.Swept the rule's consumption radius rather than the edited package:
regionKindhas no other reader in this repo outsidepackages/spec(whose own tests already carry the new shape, landed with PR #15227) and the docs reference, which is generated from the spec describe and already regenerated there. The objectuiFlowRunsPanelgrouping key is out of scope by the card and gated on a spec release.Also in the diff
.changeset/contained-failure-visibility.md(the pending #14456 entry) ended with "parallelbranch tagging is unchanged." That was true of #14456 and is falsified by this change; both entries are pending, so shipping them unedited puts a flat contradiction in one release note. Trimmed to say it was unchanged by that change and to point at this one. Nothing else in that file is touched.Changeset
.changeset/region-tagger-branch-key.md,@objectstack/service-automation: minor— notskip-changesetshape. The engine publishes different structured data into the run step log:branchappears, anditerationchanges meaning on aparallel-branchstep. That is observable to any flow author or run-observability consumer, and the exportedStepLogEntrygains a key.minorrather thanmajorto match the contract half of the same ruling, which shipped its declaration change that way; the entry states how to read a run recorded before the change, since nothing is migrated or defaulted.Checklist
docs/qa/platform-checklist/areas/automation.json,automation.flow-run-step-nesting-> revision 3: theloop { parallel }clause the card asks for, with the three-part verify (branch: 0survives; every(iteration, branch)pair appears once — a constantiterationper branch is what the old engine produced, so counting distinct values alone does not catch it; the container step carries the row and nobranch).Its fixture does not exist. showcase declares
loop,parallelandtry_catchas three separate flows and nests none in another, so the clause scoresblocked(fixture)until one lands. Filed as #16356 and recorded in the item'sfixtures.requiresrather than left silent — an absent clause is what let this overload sit unmeasured, so the clause is written now and the gap is named in the open. #16356 is not addressed here.What was run, and on which tree
The gate family was derived mechanically —
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands, from the tool's own merge-base change set, plus the four rosters it flags as kept in a directory one of these paths is in. Every exit code captured after a single redirection, never through a pipe.e64991ffa— the full union, 62/62 exit 0. On the first pass two came back exit 3PREREQUISITE NOT MET(check:dual-build-cjs-loads,check:type-check-debt, both naming an unbuiltdist/). Those are neither a pass nor a finding: the prerequisite was satisfied (turbo run build --filter='./packages/*' --filter='./packages/*/*') and both re-run as real readings.pnpm lint(eslint . --no-inline-config, whole repo, not narrowed): exit 0.pnpm --filter @objectstack/service-automation test: 121 files / 1426 tests passed.typecheck(tsc --noEmit+check:test-typecheck): exit 0.93fa7157b— head. Its only delta from the above is one prose string insideautomation.json(an unsupported "through two releases" claim, dropped). Re-run there: every gate that reads that file (check:platform-checklist,check-platform-checklist-watchdog,check:nul-bytes,check:doc-authoring,check:pm-governed-prose,check-affected-docs,check-drift-comment,check:keyed-text-bounds,check:pm-label-desc-cap) plus the ratchet family (check:type-check-debt,check:type-check-coverage,check:dual-build-cjs-loads,check:skills-token-ratchet,check:pm-skill-ratchet) — all exit 0.🤖 Generated with Claude Code
https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y
Generated by Claude Code